-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[automatic failover] Implement async creation support for StatefulRedisMultiDbConnection on MultiDbClient #3600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[automatic failover] Implement async creation support for StatefulRedisMultiDbConnection on MultiDbClient #3600
Conversation
- add tests for thread local ClientOptions - add async tracking to StatusTracker
- polish
- revisit tests
355db5b to
c47d840
Compare
68d5b36 to
78a1f99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ The following Jit checks failed to run:
- static-code-analysis-semgrep-pro
#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.
More info in the Jit platform.
ggivo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Other than what we discussed, that it is better not filter out unhealthy connections, I added some questions for my understanding.
src/test/java/io/lettuce/core/failover/MultiDbClientConnectAsyncIntegrationTests.java
Show resolved
Hide resolved
src/test/java/io/lettuce/core/failover/MultiDbClientConnectAsyncIntegrationTests.java
Show resolved
Hide resolved
5ff0966
into
redis:feature/automatic-failover-1
Add Asynchronous Connection API for Multi-Database Client
Overview
This PR introduces a new asynchronous connection API (
connectAsync) for theMultiDbClient, enabling non-blocking connection establishment to multiple Redis databases with parallel health checking and intelligent database selection.Motivation
The existing
connect()method is synchronous and blocks until all database connections are established and health checks complete. This can cause significant delays in applications that need to establish connections to multiple Redis databases, especially in scenarios where:Changes
1. New Asynchronous Connection API
Added
MultiDbClient.connectAsync(RedisCodec<K, V> codec)MultiDbConnectionFuture<K, V>(extendsBaseConnectionFuture)2. New
MultiDbAsyncConnectionBuilderClassCreated a new package-private class that encapsulates all async connection orchestration logic:
Key Methods:
connectAsync()- Main entry point for async connection creationcreateRedisDatabaseAsync()- Creates individual database connections asynchronouslyhandleDatabaseFutures()- Orchestrates collection and health checkingcollectDatabasesWithEstablishedConnections()- Implements partial success patterncollectHealthStatuses()- Waits asynchronously for all health statusesFeatures:
3. Enhanced
StatusTrackerwith Async SupportAdded
waitForHealthStatusAsync(RedisURI endpoint)CompletableFuture<HealthStatus>HealthStatusListenerClientResources.eventExecutorGroup()AtomicBooleanto prevent race conditionsUpdated Constructor:
ClientResourcesparameter for accessing the event executorMultiDbClientImplto passClientResources4. Refactored
MultiDbClientImplChanges:
createMultiDbConnection()methodStatusTrackerinstantiation to includeClientResourcesin bothconnect()andconnectPubSub()methodsconnectAsync()usingMultiDbAsyncConnectionBuilderconnect()methodconnect()method explaining blocking behavior5. New
BaseConnectionFutureClassCreated a new abstract base class for connection futures that provides protection against event loop deadlocks:
Purpose:
CompletionStage<T>andFuture<T>interfacesKey Features:
ForkJoinPool.commonPool()as default executorwrap()method for subclasses to wrap new futuresExample Problem Solved:
6. New
MultiDbConnectionFutureClassCreated a specialized connection future for multi-database connections:
Extends:
BaseConnectionFuture<StatefulRedisMultiDbConnection<K, V>>Key Methods:
from(CompletableFuture<...> future)- Static factory methodfrom(CompletableFuture<...> future, Executor executor)- Static factory with custom executorwrap(CompletableFuture<U> future)- Implementation of abstract method from base classFeatures:
BaseConnectionFutureMultiDbClient.connectAsync()7. Minor Visibility Change
RedisDatabaseImpl.getConnection():publicto package-private8. Comprehensive Test Coverage
Unit Tests (
MultiDbAsyncConnectionBuilderUnitTests):Integration Tests:
9. Updated Test Tags
Standardized test tags across all test files to use
TestTags.UNIT_TESTandTestTags.INTEGRATION_TESTconstants instead of string literals:CircuitBreakerMetricsIntegrationTestsDatabaseCommandTrackerUnitTestsDatabaseEndpointCallbackTestsDatabasePubSubEndpointTrackerTestsHealthCheckIntegrationTestsMultiDbAsyncConnectionBuilderUnitTests(new)Technical Details
Connection Flow
Partial Success Pattern
The implementation supports partial success:
RedisConnectionExceptionwith all failures as suppressed exceptionsRedisConnectionExceptionAsync Health Check Implementation
The
StatusTracker.waitForHealthStatusAsync()method uses an event-driven approach:HealthStatusListenerfor the endpointClientResources.eventExecutorGroup()to schedule timeoutAtomicBooleanto prevent double removalDeadlock Prevention
The
MultiDbConnectionFuture(viaBaseConnectionFuture) solves a critical problem with async connection handling:Problem: When using plain
CompletableFuture, callbacks can execute on Netty event loop threads. If a callback calls a blocking sync operation (likeconn.sync().ping()), it blocks the event loop thread, causing a deadlock.Solution:
BaseConnectionFutureforces all callbacks to execute on a separate thread pool (ForkJoinPool.commonPool()by default, orClientResources.eventExecutorGroup()forMultiDbConnectionFuture). This ensures:Implementation Details:
exceptionally()method is safe as-is (only runs on exception, doesn't block)API Usage
Breaking Changes
None. This is a purely additive change that maintains full backward compatibility.
Documentation
MultiDbClientinterfaceconnect()method to clarify blocking behaviorBaseConnectionFutureandMultiDbConnectionFutureFiles Changed
New Files
src/main/java/io/lettuce/core/BaseConnectionFuture.java(311 lines)src/main/java/io/lettuce/core/failover/MultiDbConnectionFuture.java(85 lines)src/main/java/io/lettuce/core/failover/MultiDbAsyncConnectionBuilder.java(432 lines)src/test/java/io/lettuce/core/failover/MultiDbAsyncConnectionBuilderUnitTests.java(392 lines)Modified Files
src/main/java/io/lettuce/core/failover/MultiDbClient.javasrc/main/java/io/lettuce/core/failover/MultiDbClientImpl.javasrc/main/java/io/lettuce/core/failover/RedisDatabaseImpl.javasrc/main/java/io/lettuce/core/failover/StatusTracker.javasrc/test/java/io/lettuce/core/failover/CircuitBreakerMetricsIntegrationTests.javasrc/test/java/io/lettuce/core/failover/DatabaseCommandTrackerUnitTests.javasrc/test/java/io/lettuce/core/failover/DatabaseEndpointCallbackTests.javasrc/test/java/io/lettuce/core/failover/DatabasePubSubEndpointTrackerTests.javasrc/test/java/io/lettuce/core/failover/HealthCheckIntegrationTests.javaTesting
All tests pass successfully:
MultiDbAsyncConnectionBuilder